Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support Composite types #303

Merged
merged 40 commits into from
Jan 9, 2024
Merged

Conversation

Tigrov
Copy link
Member

@Tigrov Tigrov commented Jul 26, 2023

Q A
Is bugfix?
New feature? ✔️
Breaks BC?
Fixed issues #290

@what-the-diff
Copy link

what-the-diff bot commented Jul 26, 2023

PR Summary

  • Addition of CompositeExpressionBuilder.php

The new file CompositeExpressionBuilder.php has been introduced. It contains code to create expressions for CompositeExpression which caters to PostgreSQL Server.

  • Enhancements to ColumnSchema.php

In ColumnSchema.php, some new functions have been added and existing ones enhanced to better manage composite type columns. These include methods to set and retrieve column properties, and to accurately handle the typecasting of array and non-array values for such columns.

  • Inclusion of CompositeExpression.php

A new file, CompositeExpression.php, is added to facilitate the representation of composite SQL expressions.

  • Modification of DQLQueryBuilder.php

Changes in DQLQueryBuilder.php allow using the newly added CompositeExpressionBuilder to create expressions for CompositeExpression.

  • Improvements to Schema.php

The Schema.php file has received changes to allow composite type management. This includes defining the 'composite' abstract column type, setting the column type to 'composite' where applicable, and ensuring composite type columns return the correct PHP type.

  • Addition of tests in ColumnSchemaTest.php

Tests for managing composite type columns are integrated into ColumnSchemaTest.php.

  • Introduction of CompositeParser.php

A new file CompositeParser.php is added, which converts composite types from PostgreSQL to PHP arrays.

  • Addition of CompositeParserTest.php

Tests for the new CompositeParser class have been added in CompositeParserTest.php.

  • Updates in SchemaProvider and pgsql.sql

In the file tests/Provider/SchemaProvider.php, a new array with composite types for the test_composite_type table have been added. The tests/Support/Fixture/pgsql.sql file has been updated to create and drop types and tables related to these new composite types.

@codecov
Copy link

codecov bot commented Jul 26, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (c4f1271) 100.00% compared to head (a5e1dbb) 100.00%.

Additional details and impacted files
@@             Coverage Diff              @@
##              master      #303    +/-   ##
============================================
  Coverage     100.00%   100.00%            
- Complexity       187       242    +55     
============================================
  Files             13        16     +3     
  Lines            566       696   +130     
============================================
+ Hits             566       696   +130     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Tigrov Tigrov marked this pull request as ready for review July 29, 2023 11:43
src/Schema.php Outdated Show resolved Hide resolved
src/Schema.php Show resolved Hide resolved
src/Composite/CompositeParser.php Outdated Show resolved Hide resolved
src/Composite/CompositeParser.php Show resolved Hide resolved
Tigrov and others added 2 commits August 1, 2023 16:28
Co-authored-by: Sergei Predvoditelev <[email protected]>
@Tigrov
Copy link
Member Author

Tigrov commented Aug 1, 2023

There are 2 options:

  1. Support missing values ['value' => 10] (missing values will be filled with default values)
    and values without field names [10, 'USD'] (field names will be filled from $columns property of ColumnSchema)
    The result before insert to db will be ['value' => 10.0, 'currency_code' => 'USD']
  2. Support only full filled field names and values for compatibility in other DBMS (after converting to json)

Currently the first option is implemented.

$command->insert('product', ['price' => ['value' => 10]])->execute();
$command->insert('product', ['price' => [10, 'USD']])->execute();
$command->insert('product', ['price' => ['value' => 10, 'currency_code' => 'USD']])->execute();

@terabytesoftw
Copy link
Member

There are 2 options:

  1. Support missing values ['value' => 10] (missing values will be filled with default values)
    and values without field names [10, 'USD'] (field names will be filled from $columns property of ColumnSchema)
    The result before insert to db will be ['value' => 10.0, 'currency_code' => 'USD']
  2. Support only full filled field names and values for compatibility in other DBMS (after converting to json)

Currently the first option is implemented.

$command->insert('product', ['price' => ['value' => 10]])->execute();
$command->insert('product', ['price' => [10, 'USD']])->execute();
$command->insert('product', ['price' => ['value' => 10, 'currency_code' => 'USD']])->execute();

We could have both behaviors, so as not to break the behavior with Yii2, one of our ideas is that by migrating to Yii2 in Db and AR, it could be done easily.

@vjik
Copy link
Member

vjik commented Aug 1, 2023

Support only full filled field names and values for compatibility in other DBMS (after converting to json)

JSON can have any structure, but don't support default values (at least in MySQL default value can be null only). Also we don't know composite type column schema. Users that want use composite type abstraction globally must use full filled fields in their code.

On the other hand user can use directly db-pgsql, and in this case he must have the ability to use benefits of composite type in PostgreSQL.

So I prefer first option that currently imlemented.

We could have both behaviors, so as not to break the behavior with Yii2, one of our ideas is that by migrating to Yii2 in Db and AR, it could be done easily.

This options contradict each other and cannot be implemented simultaneously.

@terabytesoftw terabytesoftw requested a review from darkdef August 1, 2023 18:54
@Tigrov
Copy link
Member Author

Tigrov commented Aug 4, 2023

We could have both behaviors, so as not to break the behavior with Yii2, one of our ideas is that by migrating to Yii2 in Db and AR, it could be done easily.

This is a new feature and it will not prevent migration.

But if you change DBMS for example to MySQL you can use json instead of the composite type but it will work incorrect if you do not use full filled field names and values.

CHANGELOG.md Outdated Show resolved Hide resolved
src/Composite/CompositeParser.php Show resolved Hide resolved
src/Composite/CompositeParser.php Show resolved Hide resolved
src/Builder/CompositeExpressionBuilder.php Outdated Show resolved Hide resolved
Co-authored-by: Sergei Predvoditelev <[email protected]>
Tigrov added 2 commits January 9, 2024 14:07
# Conflicts:
#	tests/ColumnSchemaTest.php
#	tests/SchemaTest.php
@Tigrov Tigrov merged commit 730a00f into yiisoft:master Jan 9, 2024
32 of 35 checks passed
@Tigrov Tigrov deleted the support_composite_types branch January 9, 2024 09:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants